feat(ui): mobile bottom navigation#2602
Open
Adebesin-Cell wants to merge 24 commits intonpmx-dev:mainfrom
Open
feat(ui): mobile bottom navigation#2602Adebesin-Cell wants to merge 24 commits intonpmx-dev:mainfrom
Adebesin-Cell wants to merge 24 commits intonpmx-dev:mainfrom
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughInitialises mobile navigation links from a new composable, adds a bottom-anchored mobile bar and a teleported mobile sheet with root/docs views, removes the old MobileMenu component, introduces module-scoped mobile-nav state, adjusts header/layout sticky offsets, and updates many i18n locales and tests. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant BottomBar as MobileBottomBar
participant MobileNav as useMobileNav
participant Sheet as MobileMenuSheet
participant RootView as MobileMenuRootView
participant Router as Router
User->>BottomBar: tap menu / search / theme / back
BottomBar->>MobileNav: open()/toggle()/back()
MobileNav->>Sheet: reactive isOpen / activeView
Sheet->>Sheet: slide-in / focus-trap / scroll-lock
User->>RootView: tap docs or nav link
RootView->>MobileNav: enterView('docs') or emit close
alt docs selected
MobileNav->>Sheet: activeView = 'docs'
Sheet->>Sheet: switch to docs pane
else nav link selected
RootView->>Router: navigate(to)
Router->>MobileNav: route change -> close()
MobileNav->>Sheet: isOpen = false
end
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Adebesin-Cell
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/Header/MobileBottomBar.client.vue`:
- Around line 88-91: The aria-label for the theme toggle in the ButtonBase
component is hard-coded and needs localization; update the ButtonBase usage in
MobileBottomBar.client.vue to call the i18n translation function for both states
(use $t or the composition API/t function) instead of the static strings,
referencing the same conditional currently using colorMode.value to choose the
message, and add the corresponding translation keys (e.g., "theme.switchToLight"
and "theme.switchToDark") to the project's locale files so each locale shows the
correct accessible label.
In `@app/components/Header/MobileMenuDocsView.vue`:
- Line 9: The hard-coded user-facing label 'Docs home' inside the docsLinks
array should be localized; replace the literal label with a translation call
(e.g., t('docs.home') or this.$t('docs.home')) and ensure the component uses the
i18n composable or instance (useI18n() / this.$i18n) so the translation key
exists in your locale files; update the docsLinks declaration (and the other
occurrences that create the same link) to use the translation key instead of the
string and add the corresponding "docs.home" entry to your locales.
In `@app/components/Header/MobileMenuRootView.vue`:
- Around line 134-141: The NuxtLink rendering in MobileMenuRootView.vue omits
rel attributes for external links (those with link.target === "_blank"), which
is a security/privacy issue; update the NuxtLink to set a rel attribute for
external targets (e.g., "noopener noreferrer") — ideally compute rel from the
link object (use link.rel if provided, otherwise set "noopener noreferrer" when
link.target === "_blank") and bind it to the NuxtLink used in the v-for over
group.items so external mobileLinks get the same protection as the docs pane.
In `@app/components/Header/MobileMenuSheet.client.vue`:
- Around line 20-33: The modal show calls in handleShowConnector and
handleShowAuth run immediately after close(), which synchronously triggers
watchers that begin deactivating the focus trap while the sheet is still
animating; defer opening the new dialog until after the focus trap deactivation
by wrapping the modal.showModal() calls in nextTick() (same pattern used by
handleOpenCommandPalette) so showModal() runs on the next tick after close()
completes; also update any watchers that deactivate the focus trap (the watchers
around line 49) to ensure the timing is compatible with nextTick-based opening.
In `@app/composables/useGlobalNavLinks.ts`:
- Around line 100-108: The links array in useGlobalNavLinks.ts currently
includes an external "Docs" entry (name: 'Docs', href: NPMX_DOCS_SITE) which
causes a duplicate row because MobileMenuRootView.vue already injects a
dedicated Docs drill-down; remove (or conditionally exclude) that external Docs
item from the links array so MobileMenuRootView.vue's drill-down is the only
Docs entry rendered (look for the object with name 'Docs' and href
NPMX_DOCS_SITE in useGlobalNavLinks.ts and delete it or wrap it with a guard
that prevents inclusion when MobileMenuRootView is used).
In `@app/pages/package/`[[org]]/[name].vue:
- Around line 8-11: The readmeStickyTop computation can briefly miscompute
during SSR/hydration because useMediaQuery('(min-width: 640px)') is false on the
server; fix by treating the server/unresolved case as the desktop offset when
computing readmeStickyTop (e.g. derive a local flag like isSmResolved = typeof
window === 'undefined' ? true : isSmUp.value and use that in the computed), and
eliminate the magic 56 by extracting it to a shared constant (e.g.
SM_HEADER_OFFSET_PX) and reference that constant in readmeStickyTop and the
other components (useMediaQuery, isSmUp, readmeStickyTop, packageHeaderHeight)
so all components stay in sync.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 723149cd-1528-481c-80e7-b5788eef8589
📒 Files selected for processing (16)
app/app.vueapp/components/AppHeader.vueapp/components/Header/MobileBottomBar.client.vueapp/components/Header/MobileMenu.client.vueapp/components/Header/MobileMenuDocsView.vueapp/components/Header/MobileMenuRootView.vueapp/components/Header/MobileMenuSheet.client.vueapp/components/Package/Header.vueapp/components/Package/Skeleton.vueapp/composables/useGlobalNavLinks.tsapp/composables/useMobileNav.tsapp/pages/package/[[org]]/[name].vueapp/pages/package/[[org]]/[name]/versions.vuei18n/locales/en.jsontest/nuxt/components/Header/MobileMenu.spec.tstest/unit/app/composables/useMobileNav.spec.ts
💤 Files with no reviewable changes (2)
- test/nuxt/components/Header/MobileMenu.spec.ts
- app/components/Header/MobileMenu.client.vue
Comment on lines
+8
to
+11
| const isSmUp = useMediaQuery('(min-width: 640px)') | ||
| const readmeStickyTop = computed( | ||
| () => `${(isSmUp.value ? 56 : 0) + (packageHeaderHeight.value || 44)}px`, | ||
| ) |
Contributor
There was a problem hiding this comment.
Minor: SSR/initial-render mismatch and magic number.
useMediaQuery('(min-width: 640px)') returns false on the server and on the very first client tick before the matchMedia listener resolves. On desktop, the initial readmeStickyTop will therefore be computed as 0 + (packageHeaderHeight || 44) = 44px instead of 56 + …, which can briefly miscalculate the readme header's sticky top until hydration settles. Since the README is far below the fold this is largely cosmetic, but worth noting.
Also, 56 duplicates the sm:top-14 Tailwind value (3.5rem = 56px) used in Package/Header.vue L238, Package/Skeleton.vue L22, and versions.vue L274. Consider extracting to a shared constant (e.g. SM_HEADER_OFFSET_PX) so all four stay in lockstep if the header height ever changes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/pages/package/`[[org]]/[name].vue around lines 8 - 11, The
readmeStickyTop computation can briefly miscompute during SSR/hydration because
useMediaQuery('(min-width: 640px)') is false on the server; fix by treating the
server/unresolved case as the desktop offset when computing readmeStickyTop
(e.g. derive a local flag like isSmResolved = typeof window === 'undefined' ?
true : isSmUp.value and use that in the computed), and eliminate the magic 56 by
extracting it to a shared constant (e.g. SM_HEADER_OFFSET_PX) and reference that
constant in readmeStickyTop and the other components (useMediaQuery, isSmUp,
readmeStickyTop, packageHeaderHeight) so all components stay in sync.
Contributor
Author
There was a problem hiding this comment.
Agreed it's cosmetic — the README is well below the fold so the first-tick value isn't visible. Happy to extract the 56 into a shared SM_HEADER_OFFSET_PX constant as a follow-up; didn't want to widen scope in this PR since the other call sites already use sm:top-14 directly. Leaving as-is for now.
Contributor
There was a problem hiding this comment.
@Adebesin-Cell, sounds reasonable — the cosmetic risk is minimal given the scroll distance, and keeping the PR scope tight makes sense. Would you like me to open a follow-up issue to track extracting 56 into a shared SM_HEADER_OFFSET_PX constant across the four call sites?
🧠 Learnings used
Learnt from: ShroXd
Repo: npmx-dev/npmx.dev PR: 2025
File: app/pages/package/[[org]]/[name].vue:210-211
Timestamp: 2026-03-15T04:00:46.905Z
Learning: In app/pages/package/[[org]]/[name].vue, the `isVersionsRoute` computed flag intentionally gates only template rendering (not data fetches) for the `package-versions` child route. Skipping data fetches on the versions route would cause them to never re-fire on navigation back to the main package page, because `immediate` is evaluated once at composable setup time. All heavyweight fetch/composable initialisation therefore runs unconditionally; only the rendered output is suppressed via `v-if="!isVersionsRoute"`. Do not flag this as a performance issue or suggest conditionally blocking fetches for this route.
Learnt from: btea
Repo: npmx-dev/npmx.dev PR: 1739
File: app/pages/package-code/[[org]]/[packageName]/v/[version]/[...filePath].vue:259-262
Timestamp: 2026-02-28T07:17:28.172Z
Learning: In app/pages/package-code/[[org]]/[packageName]/v/[version]/[...filePath].vue, the scroll-to-top button for the code preview container should be shown on both desktop and touch devices (gated only by scroll position using SCROLL_TO_TOP_THRESHOLD), not restricted to touch devices. This is different from the window-level ScrollToTop.client.vue component which is touch-device-only, because the code container scroll-to-top helps users navigate long code files where keyboard shortcuts are less practical for container-specific scrolling.
Learnt from: alexdln
Repo: npmx-dev/npmx.dev PR: 838
File: app/pages/package/[...package].vue:445-449
Timestamp: 2026-02-03T13:59:33.392Z
Learning: The copy button pattern in app/pages/package/[...package].vue may be made into a reusable component or pattern in the future, but currently it's acceptable to keep it inline with the CSS-only approach for smooth animations.
Learnt from: alexdln
Repo: npmx-dev/npmx.dev PR: 1648
File: app/pages/package/[[org]]/[name].vue:276-283
Timestamp: 2026-02-25T21:12:53.081Z
Learning: In app/pages/package/[[org]]/[name].vue, the hasServerContentOnly shallowRef is intentionally NOT reset in the app:suspense:resolve hook, unlike isSpaFallback and isHydratingWithServerContent. This persistence is by design to maintain server-rendered content state beyond initial hydration.
Learnt from: graphieros
Repo: npmx-dev/npmx.dev PR: 2087
File: app/pages/package/[[org]]/[name].vue:664-669
Timestamp: 2026-03-16T11:41:40.154Z
Learning: In `app/pages/package/[[org]]/[name].vue`, the condition `v-else-if="installSize?.totalSize"` intentionally treats a zero value as falsy, causing a zero total install size to display `(-)` instead of a formatted `(0 B)`. This is by design per maintainer confirmation (PR `#2087`). Do not flag this as a bug or suggest an explicit nullish check.
Learnt from: floatDreamWithSong
Repo: npmx-dev/npmx.dev PR: 2009
File: app/pages/package/[[org]]/[name].vue:459-464
Timestamp: 2026-03-10T01:52:11.239Z
Learning: In npmx-dev/npmx.dev, the `hasZeroDirectDependencies` computed in `app/pages/package/[[org]]/[name].vue` intentionally checks only `displayVersion.value.dependencies` and NOT `peerDependencies` or `optionalDependencies`. This is by design per issue `#1900` which states "Only dependencies are counted." `peerDependencies` and `optionalDependencies` are excluded because they do not directly inflate node_modules in the same way runtime dependencies do.
Learnt from: graphieros
Repo: npmx-dev/npmx.dev PR: 2032
File: app/components/Package/TrendsChart.vue:1356-1361
Timestamp: 2026-03-13T12:43:19.185Z
Learning: In `app/components/Package/TrendsChart.vue`, the asymmetry between `chartHeight` and the wrapper height class is intentional. `chartHeight` only reduces (from 600 to 494) when `showCorrectionControls.value && props.inModal` is true, but the wrapper height class (Lines ~1857-1863) compacts from `h-[567px]` to `h-[491px]` whenever `showCorrectionControls` is true regardless of `props.inModal`. Do not flag this as a bug or suggest aligning both conditions to `shouldCompactChart`.
Learnt from: graphieros
Repo: npmx-dev/npmx.dev PR: 2388
File: app/composables/useChartWatermark.ts:94-96
Timestamp: 2026-04-05T11:57:15.336Z
Learning: In `app/composables/useChartWatermark.ts`, the `drawNpmxLogoAndTaglineWatermark` function intentionally multiplies `npmxLogoWidthToHeight` (2.64) by `sizeRatioLogo` when scaling the watermark logo. The author (graphieros) confirmed this behavior is intentional and works correctly. Do not flag this as a bug or suggest keeping `npmxLogoWidthToHeight` as a constant when `sizeRatioLogo` is applied.
Learnt from: serhalp
Repo: npmx-dev/npmx.dev PR: 2159
File: app/composables/useCommandPalette.ts:14-15
Timestamp: 2026-03-24T12:14:15.142Z
Learning: In npmx-dev/npmx.dev (`app/composables/useCommandPalette.ts`), the module-level Maps (`queryOverrideRegistry`, `actionRegistry`, `actionIdsByScopeId`, `packageContextOpenRegistry`) are written to from server-capable pages during SSR setup, but they are **only read** by `useCommandPaletteCommands.ts`, which is consumed exclusively by `CommandPalette.client.vue`. Because the registry is never read server-side, there is no SSR cross-request contamination risk. Do not flag these registries for missing `import.meta.client` guards on write paths.
Learnt from: jellydeck
Repo: npmx-dev/npmx.dev PR: 904
File: app/components/Package/AccessControls.vue:253-253
Timestamp: 2026-02-04T05:34:20.527Z
Learning: In the npmx.dev project, ensure that focus-visible styling for button and select elements is implemented globally in app/assets/main.css with the rule: button:focus-visible, select:focus-visible { outline: 2px solid var(--accent); outline-offset: 2px; border-radius: 4px; }. Do not apply per-element inline utility classes like focus-visible:outline-accent/70 on these elements in Vue templates or components (e.g., AccessControls.vue). Rely on the global rule for consistency and maintainability; only use inline focus-visible utilities when styling non-button/select elements or in exceptional cases outside the global scope.
Learnt from: jellydeck
Repo: npmx-dev/npmx.dev PR: 904
File: app/components/Package/Versions.vue:332-332
Timestamp: 2026-02-04T05:34:54.335Z
Learning: In the npmx.dev project, focus-visible styling for buttons and selects is applied globally via main.css with: button:focus-visible, select:focus-visible { outline: 2px solid var(--accent); outline-offset: 2px; border-radius: 4px; }. Therefore, individual buttons or selects in Vue components should not rely on inline focus-visible utility classes like focus-visible:outline-accent/70. Ensure components do not add per-element focus-visible utilities; rely on the global rule. If a component seems to require a different focus state, adjust only through global CSS or maintain accessibility with native focus styles, rather than inserting inline utility classes.
Learnt from: userquin
Repo: npmx-dev/npmx.dev PR: 900
File: app/pages/cookie-policy.vue:6-14
Timestamp: 2026-02-05T03:17:30.849Z
Learning: In Nuxt projects using nuxtjs/i18n v8, rely on the auto-imported $t() in script setup (including in useSeoMeta and defineOgImageComponent) and do not destructure t from useI18n(). This pattern should apply across all page components under app/pages (e.g., index.vue, about.vue, settings.vue, search.vue, cookie-policy.vue). Ensure you consistently use the auto-imported $t() instead of importing or extracting t from useI18n().
Learnt from: userquin
Repo: npmx-dev/npmx.dev PR: 900
File: app/pages/cookie-policy.vue:6-14
Timestamp: 2026-02-05T03:23:42.731Z
Learning: In this Nuxt 4 project with nuxtjs/i18n v10, $t() and other globals like $n, $d are exposed in <script setup> and work inside callbacks (including those passed to useSeoMeta and defineOgImageComponent). This pattern applies across all pages (e.g., index.vue, about.vue, settings.vue, search.vue, cookie-policy.vue). Do not destructure t from useI18n(); rely on the global provided by Nuxt i18n in script setup.
Learnt from: userquin
Repo: npmx-dev/npmx.dev PR: 1170
File: app/components/Package/MetricsBadges.vue:66-66
Timestamp: 2026-02-08T15:02:02.232Z
Learning: In Vue components that use UnoCSS with the preset-icons collection, prefer colon-syntax for icons (e.g., i-carbon:checkmark) over the dash-separated form (i-carbon-checkmark). This aids UnoCSS in resolving the correct collection directly, which improves performance for long icon names. Apply this pattern to all Vue components (e.g., app/components/**/*.vue) where UnoCSS icons are used; ensure UnoCSS is configured with the preset-icons collection.
Learnt from: abbeyperini
Repo: npmx-dev/npmx.dev PR: 1049
File: app/components/Settings/Toggle.client.vue:22-29
Timestamp: 2026-02-11T00:01:33.121Z
Learning: In Vue 3.4 and later, you can use same-name shorthand for attribute bindings: use :attributeName instead of :attributeName="attributeName" when binding to a variable with the same name in scope. For example, :id is equivalent to :id="id" when an id variable exists. Apply this shorthand in .vue components (notably in Settings/Toggle.client.vue) to simplify templates. Ensure the bound variable exists and that you are using a Vue version that supports this shorthand.
Learnt from: ShroXd
Repo: npmx-dev/npmx.dev PR: 2025
File: app/components/Package/Versions.vue:533-540
Timestamp: 2026-03-15T03:59:32.483Z
Learning: In Vue 3, templates that use v-if/v-else render exactly one root element at runtime. Attributes (data-testid, title, etc.) will fall through to the active root without needing explicit prop forwarding. Do not flag attribute forwarding as a bug for this pattern; reserve review for cases where multiple roots render simultaneously or where explicit forwarding is required by design. This applies broadly to Vue single-file components using v-if/v-else.
Learnt from: graphieros
Repo: npmx-dev/npmx.dev PR: 2419
File: app/components/Compare/FacetQuadrantChart.vue:387-431
Timestamp: 2026-04-08T13:59:06.437Z
Learning: In the npmx-dev/npmx.dev project, vue-data-ui’s DOM-to-PNG export flow automatically ignores/hides any DOM elements marked with the `data-dom-to-png-ignore` attribute. Therefore, when reviewing Vue templates (including `#svg` slot content like in `app/components/Compare/FacetQuadrantChart.vue`), do not flag the absence of additional `v-if="!svg.isPrintingSvg && !svg.isPrintingImg"` guards solely on the basis that elements are already intended to be excluded from the export via `data-dom-to-png-ignore`.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
i18n/locales/bg-BG.json (1)
68-84: Many added placeholder namespaces appear unrelated to this mobile-nav PR.Blocks like
command_palette.*,vacations.*,brand.*,package.timeline,package.trends.chart_assistive_text,compare.scatter_chart.copy_alt, and the newcompare.packages.facets.categories.items.vulnerabilities.{githubStars,githubIssues,createdAt}keys don't seem tied to the bottom-bar/sheet feature described in the PR objectives. If these were added byi18n:check:fixsyncing againsten.json, that's fine; otherwise, consider trimming them from this PR to keep the diff focused on the mobile-nav changes.Also, per the AI summary the English source introduces
nav.docs_label,nav.back_to_main_menu, andnav.close_menufor the new mobile menu, but these are not present in this file. Based on learnings, missing keys in non-English locales are acceptable and tracked out-of-band, so no action is required here — just flagging for visibility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@i18n/locales/bg-BG.json` around lines 68 - 84, The bg-BG locale contains many placeholder namespaces unrelated to the mobile-nav change (e.g., command_palette.*, vacations.*, brand.*, package.timeline, package.trends.chart_assistive_text, compare.scatter_chart.copy_alt, and compare.packages.facets.categories.items.vulnerabilities.{githubStars,githubIssues,createdAt}); either remove these keys from this PR to keep the diff focused or confirm they were added automatically by your i18n sync tool and annotate that in the PR description. Keep the mobile menu keys consistent: do not add unrelated locales here, and note that nav.docs_label, nav.back_to_main_menu, and nav.close_menu may be missing in bg-BG (that’s acceptable) so no translation changes are required for those.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@i18n/locales/id-ID.json`:
- Around line 79-95: Remove the empty translation namespace objects from the
id-ID locale file by deleting the {} placeholders for the untranslated keys
(e.g., "command_palette", "compare.scatter_chart.copy_alt",
"compare.facets.items", "timeline" and any other empty namespaces reported in
the review); simply omit those keys entirely so the file only contains actual
translations and follows the project convention of excluding new/unchanged keys.
Locate the empty objects in the JSON (search for the exact keys above and other
top-level empty objects) and remove their key/value pairs, ensuring the
resulting JSON remains valid (comma placement adjusted as needed).
In `@i18n/locales/tr-TR.json`:
- Around line 76-92: Remove the empty placeholder objects added to the Turkish
locale by deleting the top-level keys that contain only {} (e.g.,
command_palette and its nested empty sub-objects, and other empty sections such
as vacations, action_bar, logo_menu, brand) from tr-TR.json so they are omitted
entirely; do not add empty {} entries back and ensure you only remove keys that
have no translated strings (leave any keys that already contain translations
intact) so the tooling can detect them as missing against en.json.
---
Nitpick comments:
In `@i18n/locales/bg-BG.json`:
- Around line 68-84: The bg-BG locale contains many placeholder namespaces
unrelated to the mobile-nav change (e.g., command_palette.*, vacations.*,
brand.*, package.timeline, package.trends.chart_assistive_text,
compare.scatter_chart.copy_alt, and
compare.packages.facets.categories.items.vulnerabilities.{githubStars,githubIssues,createdAt});
either remove these keys from this PR to keep the diff focused or confirm they
were added automatically by your i18n sync tool and annotate that in the PR
description. Keep the mobile menu keys consistent: do not add unrelated locales
here, and note that nav.docs_label, nav.back_to_main_menu, and nav.close_menu
may be missing in bg-BG (that’s acceptable) so no translation changes are
required for those.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 237320b3-da3f-4ed8-9262-fe357ed0ed42
📒 Files selected for processing (31)
i18n/locales/ar.jsoni18n/locales/az-AZ.jsoni18n/locales/bg-BG.jsoni18n/locales/bn-IN.jsoni18n/locales/cs-CZ.jsoni18n/locales/de.jsoni18n/locales/en.jsoni18n/locales/es.jsoni18n/locales/fr-FR.jsoni18n/locales/hi-IN.jsoni18n/locales/hu-HU.jsoni18n/locales/id-ID.jsoni18n/locales/it-IT.jsoni18n/locales/ja-JP.jsoni18n/locales/kn-IN.jsoni18n/locales/mr-IN.jsoni18n/locales/nb-NO.jsoni18n/locales/ne-NP.jsoni18n/locales/nl.jsoni18n/locales/pl-PL.jsoni18n/locales/pt-BR.jsoni18n/locales/ru-RU.jsoni18n/locales/sr-Latn-RS.jsoni18n/locales/ta-IN.jsoni18n/locales/te-IN.jsoni18n/locales/tr-TR.jsoni18n/locales/uk-UA.jsoni18n/locales/vi-VN.jsoni18n/locales/zh-CN.jsoni18n/locales/zh-TW.jsontest/unit/a11y-component-coverage.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- i18n/locales/en.json
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/Header/MobileBottomBar.client.vue`:
- Around line 52-56: The bottom bar can sit under iOS safe-area insets; update
the root div in MobileBottomBar.client.vue (the fixed <div> with :class and
:style="{'--mobile-bar-height': '3.5rem'}") to account for
safe-area-inset-bottom by making the custom property include
env(safe-area-inset-bottom) and adding padding-bottom:
env(safe-area-inset-bottom) (e.g. set --mobile-bar-height to calc(3.5rem +
env(safe-area-inset-bottom)) and add a style/paddedBottom binding that uses
env(safe-area-inset-bottom) with fallbacks using constant(...) if desired) so
the bar and its controls are offset above the iOS home indicator.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9cf47be9-17e1-4bb3-8571-791acf341efb
📒 Files selected for processing (5)
app/components/Header/MobileBottomBar.client.vueapp/components/Header/MobileMenuDocsView.vueapp/components/Header/MobileMenuRootView.vueapp/composables/useGlobalNavLinks.tsi18n/locales/en.json
🚧 Files skipped from review as they are similar to previous changes (4)
- app/components/Header/MobileMenuDocsView.vue
- i18n/locales/en.json
- app/composables/useGlobalNavLinks.ts
- app/components/Header/MobileMenuRootView.vue
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Moves mobile navigation to a persistent bottom bar and a partial sheet, inspired by svelte.dev.
Closes #718
Highlights
[logo] [search] [theme] [menu]. Hides on scroll down, reveals on scroll up./docs/*lands directly inside. Bar shows← Docsas a visible back affordance.Follow-ups